[PM-11516] Initial license file refactor #5002
Conversation
…tains all license properties as claims
… license properties as claims
|
New Issues
|
# Conflicts: # src/Core/Constants.cs
LaunchDarkly flag references🔍 1 flag added or modified
|
| var expires = subscriptionInfo.UpcomingInvoice?.Date?.AddDays(7) ?? entity.PremiumExpirationDate?.AddDays(7); | ||
| var refresh = subscriptionInfo.UpcomingInvoice?.Date ?? entity.PremiumExpirationDate; | ||
| var trial = (subscriptionInfo.Subscription?.TrialEndDate.HasValue ?? false) && | ||
| subscriptionInfo.Subscription.TrialEndDate.Value > DateTime.UtcNow; |
There was a problem hiding this comment.
You can probably leverage TimeProvider here.
There was a problem hiding this comment.
I'll note it for the next set of reactors in the next PR. In this one, I wanted to keep the logic as identical as possible to how it's working right now, just with the change of using a JWT
| claims.Add(new Claim(nameof(OrganizationLicenseConstants.BusinessName), entity.BusinessName)); | ||
| } | ||
|
|
||
| return Task.FromResult(claims); |
There was a problem hiding this comment.
❓ Out of curiosity, what's the purpose of this method returning a Task without doing anything asynchronously?
There was a problem hiding this comment.
This came out of a discussion I had with platform on the approach. The idea was to have a single interface that could be used to generate claims for any object. If we needed to have some async IO in some speculative implementation to calculate the value for a claim, then this would allow for that.
withinfocus
dismissed stale reviews from amorask-bitwarden and r-tome
via
51537ab
|
Had to do some quick hacking on this one -- please catch back up to |
# Conflicts: # src/Core/Constants.cs
# Conflicts: # src/Core/Constants.cs
cturnbull-bitwarden
force-pushed
the
billing/PM-11516/license-refactor
branch
from
9bf7d98 to
4547e12
Compare
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-11516
📔 Objective
This PR is the first, and most major iteration in a series of refactoring and cleaning up pull requests for our organization and user self-host licenses.
Currently, our licenses work based on an integer-based versioning system. Additionally, our cloud and self-host releases are at a minimum staggered by a couple of days, which can be even more if the self-host admins don't update immediately.
Suppose a new field is added to
OrganizationLicense, for example, and the version is incremented by one to accommodate the new field. In that case, self-host instances will break in the intervening time because they're not expecting the new field, and their resulting hash will not match the hash in the license file.To get around this, we've enabled Customer Success to generate licenses for any version current and prior using the Bitwarden Portal. While this workaround has been the status quo, we've decided to rectify this issue.
This pull request addresses the above problem by moving all current fields on
OrganizationLicenseandUserLicenseinto a new field,Tokenon each respective license. TheTokenis a string JWT containing a dictionary of claims that match the current fields, as well as a header and signature. In the above scenario, no additional versioning will be required from this point forward given that, if the self-host instance doesn't know to lookup a claim type, then it simply won't. The additional unused payload won't break the self-hosted instances ability to verify the signature using their public key.I made some attempts to minimize the number of changes in this pull request by using the existing paradigms and patterns given that licenses touch a fair number of files. In future pull requests, I'll be aiming to separate out application logic from the licenses, like
CanUseorVerifyData, into commands. Similarly, I'll be aiming to clean up or separate concerns inILicensingServiceinto commands and queries. Additionally, in a future PR I'll move billing related files to our own domain.📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes